[disconnected] Add option to perform image signature verification - #4141
[disconnected] Add option to perform image signature verification#4141drosenfe wants to merge 1 commit into
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Build failed (check pipeline). Post ✔️ openstack-k8s-operators-content-provider SUCCESS in 3h 53m 03s |
cc9445e to
3372477
Compare
|
Build failed (check pipeline). Post ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 59m 18s |
|
recheck |
3372477 to
5136acc
Compare
5136acc to
e1740a1
Compare
|
Build failed (check pipeline). Post ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 46m 57s |
e1740a1 to
565cc7d
Compare
|
Build failed (check pipeline). Post ✔️ openstack-k8s-operators-content-provider SUCCESS in 3h 10m 08s |
565cc7d to
baf984e
Compare
|
Build failed (check pipeline). Post ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 41m 01s |
Add option to the existing configure openshift cluster for disconnected deployment hook to perform image signature verification. This may be used when oc mirror v2 has mirrored both container images and their cryptographic signatures. jira: https://redhat.atlassian.net/browse/OSPRH-35167 Signed-off-by: David Rosenfeld drosenfe@redhat.com
baf984e to
99839c9
Compare
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
rabi
left a comment
rabi
left a comment
There was a problem hiding this comment.
A question on the default otherwise good for me.
| mirror_location: "{{ disconnect_working_dir }}/mirror_location" | ||
| local_registry: "{{ disconnect_working_dir }}/local_registry" | ||
| oc_mirror_cert_manager_catalog_url: "{{ cifmw_cert_manager_catalog_url | default('registry.redhat.io/redhat/redhat-operator-index:v4.18') }}" | ||
| verify_image_signatures: "{{ cifmw_disconnected_verify_image_signatures | default(false) }}" |
There was a problem hiding this comment.
Image signature verification is true by default downstream right? Otherwise one has to use insecureAcceptAnything: true in /etc/containers/policy.json for default/registry.
| delay: 30 | ||
|
|
||
| - name: Generate ClusterImagePolicies from mirror data | ||
| cifmw.general.generate_cluster_image_policies: |
| output_file = module.params.get("output_file") | ||
|
|
||
| # Red Hat public key: security.access.redhat.com/data/63405576.txt (between BEGIN and END, passed through | base64 -w0) | ||
| sigstore_key = """LS0tLS1CRUdJTiBQVUJMSUMgS0VZLS0tLS0KTUlJQ0lqQU5CZ2txaGtpRzl3MEJBUUVGQUFPQ0FnOEFNSUlDQ2dLQ0FnRUEwQVN5dUgyVExXdkJVcVBIWjRJcAo3NWc3RW5jQmtnUUhkSm5qenhBVzVLUVRNaC9zaUJvQi9Cb1NydGlQTXduQ2hiVENuUU9JUWVadURpRm5odUo3Ck0vRDNiN0pvWDBtMTIzTmNDU242N21BZGpCYTZCZzZrdWtaZ0NQNFpVWmVFU2FqV1gvRWp5bEZjUkZPWFc1N3AKUkRDRU40MkovallsVnF0K2c5K0dya2VyOFN6ODZIM2wwdGJxT2RqYnovVnhIWWh3RjBjdFVNSHN5VlJEcTJRUAp0cXpOWGxtbE1oUy9Qb0ZyNlI0dS83SENuL0srTGVnY08yZkFGT2I0MEt2S1NLS1ZENmxld1VaRXJob3AxQ2dKClhqRHRHbW1POWRHTUY3MW1mNkhFZmFLU2R5K0VFNmlTRjJBMlZ2OVFoQmF3TWlxMmtPekVpTGc0bkFkSlQ4d2cKWnJNQW1QQ3FHSXNYTkdaNC9RK1lUd3dsY2UzZ2xxYjVMOXRmTm96RWRTUjlOODVERVNmUUxRRWRZM0NhbHdLTQpCVDFPRWhFWDF3SFJDVTRkck1PZWo2Qk5XMFZ0c2NHdEhtQ3JzNzRqUGV6aHdOVDh5cGt5UytUMHpUNFRzeTZmClZYa0o4WVNIeWVuU3pNQjJPcDJidnNFM2dyWStzNzRXaEc5VUlBNkRCeGNUaWUxNU5Tekt3Znphb05XT0RjTEYKcDdCWThhYUhFMk1xRnhZRlgrSWJqcGtRUmZhZVFRc291REZkQ2tYRUZWZlBwYkQyZGs2RmxlYU1UUHV5eHRJVApnalZFdEdRSzJxR0NGR2lRSEZkNGhmVitlQ0E2M0pybzF6MHpvQk01QmJJSVEzK2VWRnd0M0FsWnA1VVZ3cjZkCnNlY3FraS95cm12M1kwZHFaOVZPbjNVQ0F3RUFBUT09Ci0tLS0tRU5EIFBVQkxJQyBLRVktLS0tLQ==""" |
There was a problem hiding this comment.
(non-blocking) suggestion: Let's move this below NO_POLICIES in upper case as a constant.
| for mirror, source in unique_pairs: | ||
| name = mirror.split("/")[-1].replace(".", "-").replace(":", "-") | ||
| policy = { | ||
| "apiVersion": "config.openshift.io/v1", |
There was a problem hiding this comment.
(non-blocking) question: DO we need to check openshift deployed version? I think in > 4.18 this is correct. If not this is not applied and we're silently not running this.
We can comment this that requires specific ocp version to run, or retrieving the info by "oc explain clusterimagepolicy --recursive | head -1" to parametrice this line, or maybe just not running the python code at all.
| "---\n" + "\n---\n".join(yaml.dump(p, sort_keys=False) for p in policies) | ||
| ) | ||
|
|
||
| print(f"Generated {len(policies)} ClusterImagePolicy objects in {output_file}") |
There was a problem hiding this comment.
(blocking) let's move this to module.log or put in a result field. This might bring runtime error of not able to parse JSON object.
| cifmw.general.generate_cluster_image_policies: | ||
| input_dir: "{{ mirror_location }}/working-dir/cluster-resources" | ||
| output_file: "{{ disconnect_working_dir }}/ClusterImagePolicies.yaml" | ||
| """ |
There was a problem hiding this comment.
(non-blocking) suggestion: Let's add the RETURN explanation.
| # Collect mirror-source pairs | ||
| pairs = [] | ||
|
|
||
| for fname in os.listdir(input_dir): |
There was a problem hiding this comment.
(blocking) suggestion: let's check this exists first so we can fail_json in case of not existing.
| else "imageTagMirrors" | ||
| ) | ||
| for entry in doc["spec"].get(key, []): | ||
| source = entry["source"] |
There was a problem hiding this comment.
(non-blocking) suggestion: I'd check if both keys exists before trying to access them.
|
|
||
| result = { | ||
| "success": False, | ||
| "changed": False, |
There was a problem hiding this comment.
(blocking) suggestion: We need to change this as True somewhere. Might be good L134?
| type: str | ||
| output_file: | ||
| description: | ||
| - Absolute path to directory when ClusterImagePolicy file is created |
There was a problem hiding this comment.
(blocking) typo: I think this should be "to file" instead "to directory"
|
|
||
| print(f"Generated {len(policies)} ClusterImagePolicy objects in {output_file}") | ||
|
|
||
| # Ensure some cluster image policies were created |
There was a problem hiding this comment.
(blocking) suggestion: let's move this to L 121, so we don't create a file the header.
|
(blocking) suggestion: Also I think this new python code suitable to have some tests located at: tests/unit/modules/test_generate_cluster_image_policies.py. |
3 participants
Add option to the existing configure openshift cluster for disconnected deployment hook to perform image signature verification. This may be used when oc mirror v2 has mirrored both container images and their cryptographic signatures.
jira: https://redhat.atlassian.net/browse/OSPRH-35167
Signed-off-by: David Rosenfeld drosenfe@redhat.com